-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
User controlled window visibility #9355
Conversation
c43311a
to
b475ad8
Compare
I still see a short white flash (1 frame?). Definitely shorter than without the code. I'm guessing it's probably because render hasn't run once yet. Might be better to use a run criteria to run only the second time the system is called? |
Yeah, it might be better to do it after Startup. Let me try that. |
I've tried putting it in PostStartup, an Update system that waits for 5 seconds and spawning a camera but for all of them I still get a 1 frame flash. I'm not entirely sure if that's something that can be fixed user side or even in bevy. Personally, I still prefer this over the current status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that what's shown in the example is better than the status quo.
looking at this issue looks like it might not be fixable vulkano-rs/vulkano#2263. Looking on some of the applications on my windows machine I notice a white flash on some when starting up (ldtk, firefox), but others seem to avoid it somehow (all the built in ones at least). So there must be some way to avoid it, but probably not worth worrying about for now. |
I don't think this is worth a dedicated example. Could you merge it with example |
I did it that way because I saw multiple people asking for the feature, but also I didn't realize the window_settings example was a thing so I'll merge them. |
Done. |
It looks like your PR is a breaking change, but you didn't provide a migration guide. Could you add some context on what users should update when this change get released in a new version of Bevy? |
@Selene-Amanita why do you consider this breaking? The default behaviour is still the same and it's only adding a new property |
Example |
1 similar comment
Example |
Well it's technically a change in API and also technically someone that doesn't use the struct update syntax would need to add the field, but yeah realistically it's a very minor change and probably doesn't need a Migration guide or anything.I forgot that the bot asked for it I didn't want to be passive aggressive ^^' |
Oh no, don't worry I didn't interpret it that way, I just didn't understand what was breaking. I guess adding a new public field could indeed break a build so I'll add a migration guide. Better safe than sorry. |
e503c8d
to
6ab0fe6
Compare
Example |
could you revert your changes on the root |
6ab0fe6
to
c9f8337
Compare
Done. Sorry, I thought I had already removed those changes😅. I've disabled toml formatting for bevy's repo so hopefully this doesn't happen again. |
# Objective In #9355 was added an import using bevy_internal. This change the import to use `bevy::window` instead of a `bevy_internal` to run the example outside of the bevy repo.
Objective
Solution
Notes
This is only user controlled. It would be nice if it was done automatically by bevy instead but I want to keep this PR simple.